Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(docs): Adds /no-iframe/ flag to topics/ README's videos we don't… #1380

Closed

Conversation

mfdebian
Copy link
Collaborator

@mfdebian mfdebian commented May 26, 2023

… want the parser to turn into iframes

@unjust este es un PR que va en conjunto con este otro PR a tu rama de transformar links de tu fork del repo de currículum-parser, entre ambos solucionan el issue que habíamos conversado sobre no mostrar videos en ciertas circunstancias, como en listas, funciona simplemente agregando un flag (string) /no-iframe/ al final del atributo título de cada link y luego agregando esa condición a la función que transforma los links en iframes.

busqué todos los links que tuvieran la string "youtu" y "vimeo" y que fueran parte de un link en markdown que estuviera en una lista utilizando la siguiente expresión regular:

(?:^[*-]\s*\[[^\]]+\]\((?:[^)]*(youtu|vimeo)[^)]*)\)|^\|\s*\[[^\]]+\]\((?:[^)]*(youtu|vimeo)[^)]*)\))

espero no se me hayan escapado muchos otros casos 😁

entonces las listas que se veían así:

Screenshot_2023-05-26_15-38-49

con estos cambios ya se ven así:
Screenshot_2023-05-26_15-39-45

@unjust
Copy link
Contributor

unjust commented Jun 27, 2023

No he tenido chance a revisar, pero quiero pensar cual seria mejor - usar /no-iframe/ o usar /with-iframe/ - dependente en cual es el caso mas comun.

@unjust unjust self-requested a review June 27, 2023 16:25
@mfdebian
Copy link
Collaborator Author

Estoy 99% seguro (lo revisé antes de mandar este PR) de que el caso más común y esperado por usuarias (sobre todo cuando no somos ni tú no yo 😆 ) es que al agregar el link de un video, el video aparezca, en cambio, en raras ocasiones, el video es simplemente parte de una lista de links, que son justo los casos que se atacan en este PR! Y es por eso que opté por dejar como default la opción de mostrar el video, y en cambio, cuando sólo quieres que sea un link que es parte de una lista de links, tener que tomarse la molestia de agregar el title con la string /no-iframe/ para asegurar ese comportamiento! 😊

@mfdebian mfdebian changed the base branch from main to next July 10, 2023 19:47
@unjust unjust closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants